Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make a default global location for custom user templates #1028

Merged
merged 13 commits into from
Jul 28, 2019

Conversation

t-makaro
Copy link
Contributor

@t-makaro t-makaro commented May 19, 2019

Right now there are some barriers for users to configure nbconvert with custom templates. This PR makes exporters search for custom templates within the Jupyter configuration directory (~/.jupyter/nbconvert/templates). This gives users a nice place to store their custom templates without needing to alter TemplateExporter.template_path in order to have a location where templates are globally accessible.

I also added some documentation about this new location for storing templates, and I added some detail to documentation around jinja and LaTeX.

The configuration directories for the templates are automatically created. I tried my best to follow what Jupyter notebook/jupyter_core/traitlets does for creating directories like these.

@t-makaro t-makaro requested a review from mpacer May 19, 2019 20:41
@t-makaro t-makaro requested a review from MSeal May 19, 2019 23:56
@SylvainCorlay
Copy link
Member

@t-makaro @maartenbreddels it could be interesting for nbconvert to adopt a template system similar to that of voila, where a template and corresponding resources are placed into a subdirectory of share/jupyter/voila (here it would be share/jupyter/nbconvert).

Copy link
Contributor

@MSeal MSeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So many +1's. This is a nice quality of life improvement. @SylvainCorlay do you believe we should do more now from your comment or would you be good with us merging this and maybe extending the idea later?

@SylvainCorlay
Copy link
Member

@MSeal @t-makaro I am +1 on a default global location for templates, but not in the configuration directory. IMO, it should be in the share directory instead, so that any third-party package is able to install a template for nbconvert by placing assets under e.g. PREFIX/share/jupyter/nbconvert.

Jupyter already has the machinery in jupyter_core to pick up these assets with a standard precedence. See the output of jupyter_path for nbconvert on my mac:

Screen Shot 2019-05-25 at 8 44 42 PM

@SylvainCorlay
Copy link
Member

This would play much better with template managers as well. Would you guys like to have a quick chat with us about this?

We have put some thoughts on how we could make this work with @maartenbreddels, also with the lessons learned from other extension points such as widgets.

@maartenbreddels
Copy link
Collaborator

Yes, I think it would be great if we can align these two systems. It would be great if people using voila and nbconvert only have to learn 1 system.
What is nice about how voila works, is that you can pip install a template, no post-install script is needed.

@MSeal
Copy link
Contributor

MSeal commented May 26, 2019

I'd be down for a quick chat on the topic. What's easiest tech / time for folks? We have a zoom channel we use for nteract weekly syncs that's readily available.

@maartenbreddels
Copy link
Collaborator

@MSeal that's a good idea, maybe thursday or friday would be possible?
I discussed with @SylvainCorlay some ideas for how to lay out templates on the filesystem for voila, and we expect also some discussion next week (Dashboarding workshop)

I also have some plans for voila to allow progressive loading (write out HTML ASAP to the client so it can render and fetch resources in parallel):
voila-dashboards/voila#133
Today, I was discussing with @rgbkrk that it's also useful for papermill. If a notebook takes ~30 minutes to run, it's useful to see that nbconvert/papermill is actually writing out results, and also show them.

In an ideal world, we would have:

  • pip installable templates
  • template-files can jinja-extend other template-files in other templates (directories) even in the filename is the same in an intuitive way.
  • most would work for nbconvert and voila
  • they render progressively (write to file/socket as soon as possible)

@t-makaro
Copy link
Contributor Author

@MSeal, @maartenbreddels Any time on thursday would work for me.

@MSeal
Copy link
Contributor

MSeal commented May 28, 2019 via email

@MSeal
Copy link
Contributor

MSeal commented May 29, 2019

2:30 PDT at https://zoom.us/j/795404957 then (let me know if another time would be better)

@maartenbreddels
Copy link
Collaborator

2:30 PDT is 23:30 (CEST), which is currently too late for me. Sunday late (20:00-22:30 window in CEST) might be an option for me, but cannot speak for @SylvainCorlay. Between now and an hour from now is also possible.

@MSeal
Copy link
Contributor

MSeal commented May 30, 2019

I think I would be free 20:00 CEST (18:00 UTC), I have plans for that day that might collide. Would a little earlier be possible? Maybe 19:00 CEST (17:00 UTC) on Sunday?

@mpacer
Copy link
Member

mpacer commented May 30, 2019

I could make that time (19:00 CEST (17:00 UTC) on Sunday) work.

But perhaps we could schedule a time to chat as part of the workshop next week? It seems deeply apropos.

@t-makaro
Copy link
Contributor Author

I can make 17:00 UTC on Sunday work too, or anytime Tuesday/Wednesday next week. Some other times next week may work as well, but I'm not sure yet.

@MSeal
Copy link
Contributor

MSeal commented Jun 1, 2019

17:00 UTC Sunday then?

@maartenbreddels
Copy link
Collaborator

I'll arrive at my stay in Paris at ~17:00 UTC, so I might be a bit later, 17:15 at the latest I think. I'll leave it up to you if you want to start later or wait for me.

@MSeal
Copy link
Contributor

MSeal commented Jun 2, 2019

A couple of us are on the call now fyi

@SylvainCorlay
Copy link
Member

Sorry I cannot make it (on a train with a terrible connection) but I trust maarten to present the approach in voila.

We did some brainstorming about this at the kernels workshop.

@t-makaro t-makaro changed the title make a default global location for custom user templates [WIP] make a default global location for custom user templates Jun 2, 2019
@t-makaro
Copy link
Contributor Author

t-makaro commented Jun 2, 2019

It seems that we will make some template changes in a few steps. The first step will be some path changes that I will update this PR with. I will add both a config directory location and a data directory location for templates.

@t-makaro
Copy link
Contributor Author

t-makaro commented Jun 5, 2019

Just one question about implementing the data files path.

Do we want just the one path returned from jupyter_core.paths.jupyter_data_dir() or all the paths from the list: jupyter_core.paths.jupyter_paths('nbconvert')? The former being the first element of the list.

For the config directory, I am only searching the one path returned from jupyter_core.paths.jupyter_config_dir(). Which I believe is sufficient for users' templates, but jupyter_core.paths.jupyter_data_dir() may not be sufficient for pip installing templates. I'm not entirely sure how use of the data directory works.

On my windows machine jupyter_core.paths.jupyter_paths('nbconvert') returns:

['C:\\Users\\<user>\\AppData\\Roaming\\jupyter\\nbconvert',
'E:\\ProgramData\\Anaconda3\\share\\jupyter\\nbconvert',
'C:\\ProgramData\\jupyter\\nbconvert']

@MSeal
Copy link
Contributor

MSeal commented Jun 6, 2019

Ideally I think we should move to "all the paths from the list" in priority ordering, much like how PYTHONPATH works.

@maartenbreddels
Copy link
Collaborator

In voila we use jupyter_paths indeed, and that's what I had in mind for nbconvert as well: https://github.com/QuantStack/voila/blob/a6d430857a0fddbf224d9999cf35ed381efce373/voila/paths.py#L44

FYI, we're gonna have a discussion about this again tomorrow.

@t-makaro t-makaro changed the title [WIP] make a default global location for custom user templates make a default global location for custom user templates Jun 8, 2019
@t-makaro t-makaro requested a review from MSeal June 18, 2019 23:23
Copy link
Contributor

@MSeal MSeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use a unittest, but given it's all config I'll leave it to you if you want to merge as is.

@SylvainCorlay
Copy link
Member

Quick question: what is the rationale for using a configuration path instead of a data path?

@maartenbreddels
Copy link
Collaborator

Could we iterate on this a bit more? We're working out the details for voila and I'd like to see how we can unite this very soon. voila-dashboards/voila#208

@t-makaro
Copy link
Contributor Author

t-makaro commented Jun 24, 2019

@SylvainCorlay A configuration directory is good for users who wish to make their own templates to configure a couple of things. Many people ask about configuration opptions that are currently only possible with templates. The config directory is a natural location for these people. Personally, I sync my configuration directory between multiple computers, so this acts as a global location for user templates.

@maartenbreddels This PR is meant for nbconvert 5.6. I will strip be ok to strip out the html and latex subdirectories for 6.0. The template extension changes and changing templates to be directories will also be 6.0.

@MSeal I added a test that creates a template file within the configuration directory and then attempts to use it. If the test passes, it's ready to merge. It not, I'd like to remove the test and merge as is. It was a little annoying to get this test to pass on windows.

@SylvainCorlay
Copy link
Member

@SylvainCorlay A configuration directory is good for users who wish to make their own templates to configure a couple of things.

But this appears to be just as simple to do with a template in a data directory. Templates don't have to be very complicated. Also, it sort-of breaks the unix-style pattern in terms of what configuration vs data directory are for.

Besides, nbconvert templates will probably soon need to be more than just a jinja template, but an entire directory with more resources. One common use case in my experience is that you may want to couple python configuration changes with the choice of a template. For example, in the case of an HTML exporter template, the CSS to be inlined in the header depends on the template... Right now it is hard-coded in the back-end implementation.

I am -1 on having templates in the configuration directory because it will be hard to support going forwards.

@t-makaro
Copy link
Contributor Author

I don't think this makes maintaining any more complicated, but it does make the user experience nicer. Templates are relatively environment independent, but the data directories are not, so I sync my configuration directory between computers but not my data directory. Configuration directory for user changes, and the data directory for developers to install data to.

For example, in the case of an HTML exporter template, the CSS to be inlined in the header depends on the template... Right now it is hard-coded in the back-end implementation. I really do consider template changes to be a configuration, and I don't think a more sophisticated template system changes that use case. A common case it to create a template and then set the configuration.py file to use that template. The current search path is cwd -> user changes -> data packages -> nbconvert templates. I might agree if the cwd was not already a part of that chain.

@SylvainCorlay
Copy link
Member

I really do consider template changes to be a configuration

I would argue that the choice of the template itself is configuration, but not the template itself.

A common case it to create a template and then set the configuration.py file to use that template.

Package managers are really bad at dealing with configuration.

@maartenbreddels
Copy link
Collaborator

I'm a bit torn between the using ~/.jupyter and not for convenience. ~/.jupyter doesn't specify what it exactly contains, but dot-directories are mostly used for configuration.

I've been checking what other software does, and according to https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html (or the more human readable version https://askubuntu.com/questions/14535/whats-the-local-folder-for-in-my-home-directory) says to use ~/.local/share/ for user data, which is what jupyter_path(..) will already give us on Linux, and ~/Library/<appname> on OSX (which is also standard).

So I have to agree with Sylvain here, using ~/.jupyter for data doesn't seem to fit what we want to do, and ~/.local/share (~/Library/ on OSX) does seem like the right choice here, and are already what jupyter_path(..) will give us.

I propose we leave out the jupyter_config_dir() since jupyter_path() will already give is a user-data directory. It will make nbconvert simpler to use (otherwise the list of path we need to document/communicate to users is just really long), and will also break less for 6.0

The current search path is cwd -> user changes -> data packages -> nbconvert templates

Is cwd in the search path? I don't see that in the code, but that could be useful for voila as well.

@MSeal
Copy link
Contributor

MSeal commented Jun 28, 2019

Alright, so where are we on action items for this PR? I think I can agree to the logic @maartenbreddels and @SylvainCorlay layed out for config / data directory choices. @t-makaro do we need to discuss more options for making your use case more convenient?

@t-makaro
Copy link
Contributor Author

t-makaro commented Jul 28, 2019

Sorry for the long delay.

Is cwd in the search path? I don't see that in the code, but that could be useful for voila as well.

@maartenbreddels template_path=List([.]) defines a list of extra paths to find templates and defaults to the cwd.

@MSeal I think I can make it work so long as the template_path list is configurable.

I believe this is ready to merge.

@MSeal
Copy link
Contributor

MSeal commented Jul 28, 2019

Awesome. Yes this look solid and we can adjust if the viola devs want to add things later.

No problem on the delay, I've similarly been our of sync with tasks.

@MSeal MSeal merged commit 48a4698 into jupyter:master Jul 28, 2019
@MSeal MSeal added this to the 5.6 milestone Jul 30, 2019
@t-makaro t-makaro mentioned this pull request Jul 30, 2019
2 tasks
@meeseeksmachine
Copy link

This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/nbconvert-5-6-0-release/1867/1

@txoof
Copy link

txoof commented Sep 30, 2020

Did this resolve the location for custom templates? If so, how is it handled where does one set the location for custom templates?

See #1428 for more details

@MSeal
Copy link
Contributor

MSeal commented Sep 30, 2020

Responded on the new issue you made

jasongrout added a commit to jasongrout/nbconvert that referenced this pull request Sep 17, 2021
Fixes jupyter#1430

Many users are reporting that nbconvert 6.x has permission errors when queried or converting files. These seem to arise when nbconvert is installed as root, then used as a user. We hypothesize that the code removed in this commit, which attempts to create template directories throughout the jupyter path hierarchy, succeeds in creating template directories when running as root. Note that it creates the template directories with permission 700. Those permissions as root mean that regular users won’t be able to read these directories, and those are the sort of permission errors we are seeing reported.

This directory creation was originally introduced in jupyter#1028, where I think it was limited to a single user configuration directory (based on the PR discussion). In jupyter#1056, this code appears to have been copied over and applied to the entire Jupyter directory hierarchy.
maartenbreddels pushed a commit that referenced this pull request Sep 17, 2021
* Do not attempt to create additional template paths.

Fixes #1430

Many users are reporting that nbconvert 6.x has permission errors when queried or converting files. These seem to arise when nbconvert is installed as root, then used as a user. We hypothesize that the code removed in this commit, which attempts to create template directories throughout the jupyter path hierarchy, succeeds in creating template directories when running as root. Note that it creates the template directories with permission 700. Those permissions as root mean that regular users won’t be able to read these directories, and those are the sort of permission errors we are seeing reported.

This directory creation was originally introduced in #1028, where I think it was limited to a single user configuration directory (based on the PR discussion). In #1056, this code appears to have been copied over and applied to the entire Jupyter directory hierarchy.

* Filter additional template paths to existing paths to be consistent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants